Skip to content

4118 better indexing for rewrite rules #4120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Jul 18, 2025

Conversation

jberthold
Copy link
Member

@jberthold jberthold commented Jul 17, 2025

Indexing is extended to

  • consider domain values
  • distinguish function and constructor symbols
  • use special indexes for internalised maps, sets, and lists

Cell indexes form a (flat) lattice which is now explicit in a less-or-equal definition.

                Anything
   ____________/   |  \_______________________________________...
  /          /     |            |           \             \
TopList..TopSet  TopVal "x"..TopVal "y"  TopCons "A"..  TopFun "f"..
  \_________| __   |   ________|____________|____________/____...
                \  |  /
                 None

Rule selection now uses a function that selects all rule indexes which are (as a product lattice) greater than the inverted term index (i.e., top element becomes bottom element), with all function indexes turned into Anything (top element) before the inversion. The new integration test shows why this is important for soundness.

This addresses cases of unevaluated functions in indexed cells where previously booster would (likely) report Stuck or (unlikely) a wrong result, while the result should be Aborted (rules were excluded but they could apply depending on what kind of term the unevaluated function would return when evaluated).

Fixes #4118

@jberthold
Copy link
Member Author

jberthold commented Jul 17, 2025

KEVM tests with insignificant slowdown

Test 4118-better-indexing-for-rewrite-rules time master-fbee2cb2d time (4118-better-indexing-for-rewrite-rules/master-fbee2cb2d) time
erc20/ds/allowance-spec.k 57.71 55.72 1.0357142857142858
erc20/hkg/transferFrom-success-2-spec.k 64.76 62.5 1.0361600000000002
benchmarks/ecrecoverloop02-sig0-invalid-spec.k 86.0 82.97 1.0365192238158372
mcd-structured/end-cash-pass-rough-spec.k 145.39 139.86 1.0395395395395393
erc20/ds/transfer-failure-1-a-spec.k 79.02 75.98 1.0400105290866015
benchmarks/ecrecoverloop00-sig0-invalid-spec.k 70.03 67.16 1.042733770101251
kontrol/test-emitcontracttest-testexpectemit-0-spec.k 60.13 57.64 1.0431991672449688
mcd-structured/flopper-dent-guy-same-pass-rough-spec.k 165.28 158.22 1.044621413222096
mcd-structured/dai-adduu-fail-rough-spec.k 61.34 58.71 1.0447964571623234
mcd-structured/flopper-dent-guy-diff-tic-not-0-pass-rough-spec.k 201.4 192.56 1.0459077690070628
erc20/ds/transfer-success-1-spec.k 65.44 62.52 1.0467050543825975
erc20/ds/transferFrom-success-2-spec.k 70.32 66.65 1.0550637659414852
benchmarks/ecrecoverloop00-sig1-invalid-spec.k 106.28 100.4 1.0585657370517927
mcd/flopper-file-pass-rough-spec.k 28.0 26.41 1.0602044680045437
TOTAL 1261.1 1207.3 1.0445622463347966

Likewise for kontrol tests:

Test 4118-better-indexing-for-rewrite-rules time master-fbee2cb2d time (4118-better-indexing-for-rewrite-rules/master-fbee2cb2d) time
NestedStructsTest.prove_fourfold_nested_struct(((((uint8,uint256),bytes32)[],bytes32))) 9.76 13.52 0.7218934911242604
MethodDisambiguateTest.test_method_call() 8.25 10.79 0.7645968489341984
ExternalLibTest.testSum() 7.97 10.33 0.771539206195547
ExpectRevertTest.testFail_expectRevert_empty() 8.31 10.66 0.7795497185741088
GasTest.testSetGas() 9.05 11.37 0.7959542656112578
ExpectRevertTest.test_expectRevert_message() 14.55 16.93 0.8594211458948613
ExpectCallTest.testExpectRegularCall() 13.71 15.9 0.8622641509433963
MockCallTestFoundry.testMockCallEmptyAccount() 22.6 25.87 0.8735987630459993
EmitContractTest.testExpectEmit() 12.38 14.17 0.8736767819336627
test_foundry_xml_report 11.84 13.47 0.8789903489235338
AssumeTest.test_assume_true(uint256,uint256) 9.42 10.63 0.8861712135465662
DynamicTypesTest.test_complex_type((uint256,bytes),bytes[]) 19.29 21.27 0.9069111424541607
MockCallTestFoundry.testMockNested() 37.52 40.81 0.9193825042881647
AddrTest.test_builtInAddresses() 8.12 8.64 0.9398148148148147
AssumeTest.testFail_assume_false(uint256,uint256) 25.8 27.39 0.9419496166484118
AllowChangesTest.testFailAllowChangesToStorage() 12.4 13.15 0.9429657794676806
AssertTest.test_assert_true_branch(uint256) 17.91 18.73 0.9562199679658302
test_foundry_show_with_hex_encoding 3.58 3.74 0.9572192513368983
LoopsTest.test_sum_10() 13.19 13.77 0.9578794480755265
StoreTest.testLoadNonExistent() 8.2 8.55 0.95906432748538
FreshBytesTest.test_symbolic_bytes_3 27.18 28.2 0.9638297872340426
PlainPrankTest.test_stopPrank_notExistent() 8.38 8.69 0.9643268124280784
AddrTest.test_notBuiltinAddress_symbolic(address) 12.09 11.64 1.0386597938144329
StoreTest.testGasStoreWarmUp() 14.55 13.94 1.043758967001435
AssertTest.test_failing_branch(uint256) 30.94 29.37 1.0534559073884917
StoreTest.testGasLoadColdVM() 10.44 9.87 1.0577507598784195
AssumeTest.testFail_assume_true(uint256,uint256) 22.95 21.69 1.058091286307054
test_foundry_merge_loop_heads 40.95 38.7 1.058139534883721
MockCallTestFoundry.testMockCalldata() 32.96 30.83 1.0690885501135259
AllowChangesTest.testFailAllowCallsToAddress() 9.14 8.54 1.0702576112412179
FreshBytesTest.test_symbolic_bytes_1 29.62 27.6 1.0731884057971015
StoreTest.testGasStoreColdVM() 10.74 9.98 1.0761523046092185
AssumeTest.test_multi_assume(address,address) 18.69 17.36 1.0766129032258065
DynamicTypesTest.test_dynamic_struct_array((uint256,bytes)[]) 21.91 20.27 1.0809077454366058
CounterTest.testSetNumber(uint256) 22.3 20.62 1.0814742967992241
Setup2Test.test_setup() 9.14 8.43 1.0842230130486359
MockCallTestFoundry.testMockGetters() 30.61 28.22 1.0846917080085046
ExpectRevertTest.test_ExpectRevert_increasedDepth() 18.93 16.92 1.1187943262411346
MockFunctionTest.test_mock_function() 33.42 29.84 1.1199731903485255
EmitContractTest.testExpectEmitLessTopics() 14.24 12.46 1.1428571428571428
PlainPrankTest.test_prank_zeroAddress_true() 22.28 19.47 1.1443246019517208
ExpectRevertTest.testFail_expectRevert_false() 16.59 14.38 1.1536856745479833
ExpectRevertTest.test_expectRevert_true() 14.9 12.82 1.1622464898595943
FreshCheatcodes.test_int128() 11.28 8.77 1.2862029646522235
ImmutableVarsTest.test_run_deployment(uint256) 97.52 64.79 1.5051705510109583
test_constructor_with_symbolic_args 42.81 26.62 1.6081893313298272
TOTAL 898.41 849.71 1.0573136717232938

@jberthold jberthold marked this pull request as ready for review July 17, 2025 05:55

syntax KItem ::= "Done" [symbol(Done)]

syntax Abcd ::= "A" | "B"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a new sort

syntax Ab ::= "A" [token]
            | "B" [token]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that, but it actually breaks the demonstration of the unsound behaviour (because the tokens get index Anything and the Vrule gets tried first). So I just corrected the indexes in the comments and left the test as-is.

@ehildenb
Copy link
Member

I love the lattice formulation, that reflects unification properties very nicely! As long as we respect the properties of unification, we can be sure our rule indexing will be sound!

@jberthold jberthold merged commit b5d3b9f into master Jul 18, 2025
6 checks passed
@jberthold jberthold deleted the 4118-better-indexing-for-rewrite-rules branch July 18, 2025 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overhaul indexing mechanisms and algebra in booster
3 participants